feat: multi-tenant organizations with a project role ladder (owner/manager/editor/viewer) - #267

…nches

Give the three access branches a Drive-style role ladder instead of raw
ok/isOwner flags: row owner → owner, shared_with email → editor, org
owner/admin → manager, plain org member → viewer. A single capability
matrix (lib/permissions.ts) maps roles to what routes may do — view,
content.edit, docs.organize, structure.manage, members.manage,
container.delete — and every project/document/review write route now
declares the capability it needs instead of hand-rolling an owner check.

This makes the ADR's 'org membership grants visibility, not ownership'
promise real: plain org members are read-only (previously the org branch
returned ok:true and most write routes gated on nothing beyond ok), and
org owner/admins can curate content (manage folders, sharing, review
structure) without being able to delete containers they don't own.

Notable tightenings, all fail-closed:
- folder rename/move/delete, doc-set/column edits on reviews, and
  clear-cells are manager+ (generalising the owner-only folder-delete
  gate that landed upstream in Open-Legal-Products#193)
- version pushes, edit resolution, chat, and review generation are
  editor+ (org viewers excluded)
- project PATCH (metadata + sharing) is manager+, so org admins can
  manage without owning; project/review DELETE stays owner-only
- GET /projects/:id and /people now go through checkProjectAccess (the
  roster previously 404'd for org members who could read the project)

Detail responses expose access_role alongside is_owner so the client
can render per-role affordances. can() is exhaustively unit-tested
(role × capability), and route suites cover the new gates.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

Org viewers could append messages to a colleague's chat — and burn LLM
credits doing it — because the existing-chat path only asked "can you
see this chat?", never "can you write to it?".

WHY THIS MATTERS

The organizations feature gives every plain org member visibility into
their org's projects: they map to the "viewer" project role
(backend/src/lib/access.ts, orgRoleToProjectRole). Visibility is the
point — but the capability matrix (backend/src/lib/permissions.ts) is
explicit that chatting is a WRITE:

    capability    | min role | covers
    content.edit  | editor   | upload documents, push versions, CHAT, ...

Creating a chat already honored that: validateAccessibleProjectId gates
new project chats on can(projectRole, "content.edit"). But POST /chat
with an existing chat_id, and POST /chat/:chatId/generate-title, gated
only on getAccessibleChat — which returned the chat whenever
checkProjectAccess(...).ok was true. That is true for org viewers. So a
viewer could:

  - append user messages into a colleague's project chat,
  - trigger LLM generation (spending the owner's configured API keys),
  - overwrite the chat's title via generate-title (an UPDATE on chats).

WHAT IS THE ACCESS-VS-CAPABILITY DISTINCTION

"Can you see this resource?" and "can you write to this resource?" are
different questions and must be answered by different checks:

  - an ACCESS check resolves whether the caller has any standing at all
    (owner / shared editor / org member) — it yields a role;
  - a CAPABILITY check asks whether that role clears the bar for the
    specific operation: can(role, "content.edit").

A route that stops after the access check silently grants its weakest
role the powers of its strongest. That is exactly the bug class here:
read-only endpoints (GET /chat/:chatId) and write endpoints (POST
/chat) shared one gate, so the gate had to be as permissive as the
reads — and the writes inherited that permissiveness.

HOW THE FIX WORKS

getAccessibleChat now returns the caller's ProjectRole along with the
chat, instead of flattening everything to "found / not found":

    type ChatAccess =
        | { ok: true; chat: AccessibleChat; projectRole: ProjectRole }
        | { ok: false };

The chat owner maps to "owner"; for project chats everyone else
inherits their project role from checkProjectAccess. Chats without a
project_id remain reachable only by their owner (unchanged).

Each caller then declares the capability it needs:

  - GET /chat/:chatId — read: any resolved role (project.view
    semantics), so org viewers keep read access. Unchanged behavior.
  - POST /chat with chat_id — write: rejects with 403 unless
    can(projectRole, "content.edit"), mirroring the new-chat path.
  - POST /chat/:chatId/generate-title — write (UPDATEs chats.title):
    same 403 gate.

403 (not 404) is correct for viewers: they are allowed to know the chat
exists — they can read it — they just cannot modify it.

Note: routes/projectChat.ts does NOT have this hole — it already gates
the entire POST on content.edit before touching any chat row.

Tests (chat.routes.test.ts) add a table-aware supabase stub so the
caller's org role can be varied, and prove: viewer POST to an existing
chat → 403 with no LLM call; viewer generate-title → 403; chat owner
and org admin (manager) still stream successfully; admin generate-title
still works; viewer GET of the same chat still returns 200.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

With two or more owners in an org, an admin could demote an owner to
member (or remove them outright), because the membership mutations
checked the ACTOR's rank but never compared it to the TARGET's.

WHY THIS MATTERS

The role ladder is owner > admin > member. updateMember and
removeMember guarded three things:

  - the actor can manage members at all (roleCanManage → owner/admin),
  - "only an owner may GRANT the owner role" (no self-escalation),
  - last-owner protection (never demote/remove the sole owner).

None of those look at who the target IS. So in an org with owners A and
B plus admin C, C could call updateMember(target=A, role="member"):
C passes roleCanManage, "member" is not "owner" so the grant check does
not fire, and B still counts as an owner so last-owner passes. Result:
an admin deposes an owner — and with both owners demoted one at a time,
an admin becomes the effective top of the org without ever holding the
owner role. That inverts the hierarchy the ladder is supposed to
encode.

WHAT IS AN ACTOR/TARGET RANK CHECK

Role-ladder authorization has two halves, and they are easy to conflate:

  1. Does the ACTOR's role permit this KIND of operation?
     ("admins may manage members")
  2. Does the ACTOR outrank-or-equal the TARGET of the operation?
     ("...but not members who outrank them")

Check (1) alone is enough for rank-neutral operations (creating a
team). Any operation aimed at another member also needs check (2),
otherwise every manager-tier role can act on the tier above it. The
existing "only an owner may grant owner" rule is the escalation half of
this idea; what was missing is the demotion half: only an owner may act
AGAINST an owner.

HOW THE FIX WORKS

Both mutations now fetch the target's role (they already did, for
last-owner counting) and add a rank guard before it:

    if (targetRole === "owner" && actorRole !== "owner")
        return { ok: false, kind: "forbidden" };

  - updateMember: an admin demoting an owner → forbidden, regardless of
    how many owners exist. Owner-on-owner changes still work.
  - removeMember: an admin removing an owner → forbidden. An owner
    leaving on their own is unaffected: self-leave implies
    actorRole === targetRole === "owner", so the guard passes and the
    existing last-owner protection still has the final say.

Since owner/admin/member is a three-rung ladder, this single condition
IS the full outrank-or-equal rule: owners outrank everyone, admins may
act on admins/members, and plain members cannot reach these functions
at all (roleCanManage already rejects them).

Tests (orgs.test.ts) seed a two-owner org so last-owner protection is
provably not what stops the attack: admin-demotes-owner → forbidden,
admin-removes-owner → forbidden, owner-demotes-owner still ok,
owner-self-leave with a second owner still ok, and the existing
last-owner cases stay green.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

Adding an org or team member by email used the Supabase admin API's
listUsers({ perPage: 1000 }) and scanned the page in JavaScript. Any
user beyond the first 1000 accounts silently resolved to null, so the
route answered "No user with that email" for a user who exists.

WHY THIS MATTERS

List-and-scan lookups against an admin API are a scalability trap that
looks fine in every dev and demo environment:

  - CORRECTNESS decays with growth. listUsers is paginated; a single
    perPage: 1000 call is only "everyone" while the instance has fewer
    than 1000 accounts. The day it crosses that line, membership adds
    start failing for exactly the newest users — with a misleading 404
    and no error anywhere in the logs.
  - COST grows linearly. Even while it still works, resolving ONE email
    means transferring up to 1000 user records and comparing each in
    process, on every add-member call — O(n) network and CPU for what
    an indexed table answers in O(log n).
  - The admin auth API is a management surface, not a query engine. It
    has no "find by email" filter here, which is the hint that
    lookup-by-attribute belongs on a queryable table with an index.

The comment above the helper claimed it "mirrors the lookup pattern in
routes/projects.ts /people" — but that route actually uses the
user_profiles-based helpers in lib/userLookup.ts, not the admin API.
The codebase already had the right tool; this route just didn't use it.

WHAT THE INDEXED LOOKUP IS

lib/userLookup.ts maintains lookups over the user_profiles table, which
stores each user's normalized (lowercased, trimmed) email and is kept
in sync on auth events (syncProfileEmail). findProfileUserByEmail is a
single indexed query:

    const { data } = await db
        .from("user_profiles")
        .select("user_id, email, display_name")
        .eq("email", normalized)
        .maybeSingle();

One row travels over the wire regardless of whether the instance has
100 users or 10 million, and "not found" is an honest answer instead of
an artifact of pagination.

HOW THE FIX WORKS

routes/orgs.ts's resolveUserIdByEmail now delegates to that helper:

    const user = await findProfileUserByEmail(db, email);
    return user?.id ?? null;

The contract is unchanged — null when no such user — so both callers
(POST /orgs/:orgId/members and POST /orgs/:orgId/teams/:teamId/members)
keep their existing 404 behavior for genuinely unknown emails, and the
stale comment is corrected. This was also the last listUsers scan in
the backend. Email normalization (trim + lowercase) lives inside the
helper, matching the semantics the old loop implemented by hand.

findProfileUserByEmail is covered by unit tests
(lib/__tests__/userLookup.test.ts: found via normalization, not-found,
and blank-input cases); the org routes have no route-level test file,
so the two-line delegation rides on that existing coverage.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

…API actually calls

WHY THIS MATTERS

PR migration 20260717_03 added the new org-membership visibility branch
("you can see rows tagged with an org you belong to") to
get_tabular_reviews_overview — but only to its legacy 3-argument
overload. The API never calls that overload. GET /tabular-review builds
nine named arguments (p_user_id, p_user_email, p_project_id, p_scope,
p_limit, p_offset, p_search_term, p_sort_key, p_sort_direction) in
lib/tabularReviewsOverview.ts, so PostgREST resolves the 9-argument
overload from 20260726_01 — which never got the org branch. Result: on
any install upgraded via migrations, org-shared reviews were readable
through the detail endpoints (access.ts allows them) yet invisible in
every list view. Fresh installs, which bootstrap from schema.sql, were
unaffected because schema.sql's 9-argument version already carries the
org branch — so fresh and migrated databases silently diverged, the
worst kind of bug: nothing errors, some deployments just show less data.

WHAT IS FUNCTION OVERLOADING IN POSTGRES

Postgres identifies a function by name AND argument types.
get_tabular_reviews_overview(text, text, text) and
get_tabular_reviews_overview(text, text, text, text, integer, integer,
text, text, text) are two completely independent functions that happen
to share a name. CREATE OR REPLACE matches on the full signature, so
replacing one overload never touches the other:

  create or replace function f(a text) ...           -- replaces f(text)
  create or replace function f(a text, b int) ...    -- separate function!

PostgREST resolves an RPC call to the overload whose named parameters
match the JSON body it received. A call with nine named keys can only
ever hit the 9-argument overload. That is why patching the 3-argument
overload in 20260717_03 was a no-op for the API.

HOW THE FIX WORKS

This migration re-declares both overloads exactly as backend/schema.sql
already defines them (the bodies are copied verbatim, not rewritten):

1. The 9-argument overload gains the two org-membership arms:
   - accessible_projects: projects whose org_id is in an org the caller
     belongs to (EXISTS against org_members) — so in-project reviews of
     org colleagues become visible;
   - visible_reviews: org-tagged reviews owned by someone else become
     visible in the global list (p_project_id is null).
2. The 3-argument overload becomes the thin wrapper schema.sql uses —
   it simply delegates to the 9-argument version with scope 'all' and an
   effectively unlimited page — replacing 20260717_03's divergent
   full-body copy. From now on there is a single source of truth for the
   visibility predicate, and this file sorts after every earlier
   overview migration, so replaying migrations from scratch also ends in
   the correct state (previously 20260726_01 sorted after 20260717_03
   and wiped the org branch on replay).

Signatures do not change, so CREATE OR REPLACE suffices — no DROP
FUNCTION, and the migration is safe to re-run. schema.sql needs no
change: it was already correct; this converges migrated installs onto
it.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

WHY THIS MATTERS

get_tabular_review_ids_overview backs GET /tabular-review/ids, the bulk
"select all matching" action: it returns id + owner for every review the
caller can see, so the client can select the whole filtered set without
fetching full payloads. For performance it does NOT delegate to
get_tabular_reviews_overview — it carries its own copy of the visibility
predicate, and its own migration (20260727_01) warns in its header:

  "If the access/visibility rules in get_tabular_reviews_overview's
   visible_reviews CTE ever change, mirror the change here too."

The organizations work changed exactly those rules — it added a third
visibility branch, "rows tagged with an org the caller belongs to",
alongside "row owner" and "shared_with email" — but this RPC was never
mirrored, neither in a migration nor in schema.sql. The user-visible
symptom is nasty because it is partial: an org member SEES a colleague's
org-shared reviews in the list, but "select all matching" silently
skips them, so bulk actions run over fewer rows than the visible
selection implies. Nothing errors; rows just go missing.

HOW THE FIX WORKS

Add the same two org-membership arms the paginated overview uses
(compare its 9-argument definition in schema.sql):

1. accessible_projects gains an EXISTS against org_members on
   p.org_id — reviews living in an org colleague's project become
   visible, exactly like email-shared projects already were:

     or (
       p.org_id is not null
       and p.user_id <> p_user_id
       and exists (
         select 1 from public.org_members m
         where m.org_id = p.org_id and m.user_id::text = p_user_id
       )
     )

2. The row filter gains the matching arm for org-tagged reviews in the
   global list (p_project_id is null), keyed on tr.org_id.

The change is made in BOTH places that define this function — edited
in place in backend/schema.sql (fresh installs bootstrap from it) and
as new migration 20260805_02 (upgrades existing installs) — with
byte-identical function bodies, so the two install paths cannot
diverge. org_members.user_id is a uuid FK to auth.users while content
tables store user_id as text, hence the ::text casts. Org membership
grants visibility only; user_id still identifies the owner.

The signature is unchanged, so the migration is CREATE OR REPLACE only
and safe to re-run.

TESTS

The org-visibility integration suite (tabularPagination.supabase.test.ts)
now seeds two real auth users in one org — the FK to auth.users means
random UUIDs won't do — makes one a plain "member", and asserts the
member sees the colleague's in-project and standalone org reviews via
the ids RPC, plus a lockstep assertion that the ids RPC returns exactly
the set the paginated overview shows, which is the drift this bug class
is about.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

…actor

WHY THIS MATTERS
The manager-gate test for POST /tabular-review/:reviewId/clear-cells was
written when the endpoint keyed cell resets on document_ids. After
rebasing onto main it hit the 400 validation branch ("row_ids is
required") before ever reaching the 403 role check it exists to prove,
so the RBAC gate went untested.

WHAT IS THE LOGICAL-ROWS REFACTOR
Main's folder-grouped tabular review work (Open-Legal-Products#274) reshaped reviews around
logical review rows: a row can represent a folder of documents, not just
one document, so cell-level operations now address rows. clear-cells
accordingly takes row_ids instead of document_ids.

HOW THE FIX WORKS
The test now sends { row_ids: ["row-1"] }, a valid payload under the
current API, so the request passes validation and exercises the intended
assertion: an org editor without structure.manage receives 403 "Only a
review manager can clear cells". No production code changes.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>